New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implement (optional) semver parsing of version #213
Conversation
Codecov Report
@@ Coverage Diff @@
## master #213 +/- ##
==========================================
- Coverage 90.76% 89.02% -1.75%
==========================================
Files 4 4
Lines 65 82 +17
Branches 9 12 +3
==========================================
+ Hits 59 73 +14
- Misses 3 5 +2
- Partials 3 4 +1
Continue to review full report at Codecov.
|
Yeah me too hehe. I'll do a review soon. Thanks for your contribution! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but my TS-fu is even worse than my JS-fu :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@radeksimko The README would also need to be updated to address this new feature. And also run npm run pre-checkin
and push changes. Thanks!
@crazy-max I believe I addressed all your comments. The implementation doesn't require GitHub token nor GitHub SDK anymore as it leverages the static JSON as discussed. PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@radeksimko LGTM!
Fixes #157
I admit my JavaScript-fu is very rusty. I welcome any feedback. Also there's perhaps more changes outside of the context of the original problem - I'd be happy to extract these into separate PRs, if necessary.